Repository navigation
Conversation
…t handling - drop symlinks whose realpath leaves the repository or has none (D-sc-03; the scanner-source-listing pin no longer lists an out-of-tree symlink) - skip unreadable package manifests with a diagnostic instead of failing the whole observation (D-sc-04) - report dangling tsconfig projectReferences as diagnostics and never follow references outside the repository (D-sc-05) - cap the Swift worker output buffer at 256 MB (D-sc-08) - resolve dependency versions only from in-tree installs (D-sc-09)
Source file names flowed unescaped into groma/relationships.md table rows; a name containing pipes, newlines, or brackets mangled the row and permanently broke every architecture load (D-sc-02). Link text, hrefs, and cells are now escaped so every row round-trips through the strict reader.
…ences - reject requests whose Host is not loopback (DNS rebinding, WEB-01) - refuse cross-origin state-changing requests while bare clients still work (WEB-02) - refuse stored code references that escape the repository at the web layer (WEB-03) - render work-island pins and statuses as text nodes, never innerHTML (WEB-04)
- catch errors in the root, export, and instructions actions (D2-3) - catch welcome-loop action failures like groma scan does (D2-4) - correct draft relation help to the required flags (D2-5) - reflect scanner errors in the welcome status bar (D2-7) - exit deterministically when a rejected close() fails during shutdown (D2-8)
GromaFileSystem.write now writes a same-directory temp file and renames it over the target, matching the existing replaceFile pattern, so an interrupted write can no longer leave a truncated, unloadable record (D2-1).
- validate task ids before spawning the CLI and quote shim arguments (D-work-01) - resync the snapshot stream past noise and malformed regions (D-work-02) - coerce degenerate task fields to the work-source contract (D-work-03) - escalate to SIGKILL when a watched CLI ignores SIGTERM (D-work-04) - order dotted task ids segment by segment (D-work-05)
Every third-party action was referenced by a floating tag while the release workflow ran with contents: write and id-token: write and committed to main through an unpinned action (D5-5). All 13 distinct actions are now pinned to their current tag commits with the tag kept as a comment.
…the code - product-model: exports ship the profile, map, flows, and read-only source inspection without task data, matching the implementation (D5-0) - component-markdown: point the package example at the real system path (D5-1) - relationship-inference: re-measured counts 119 elements, 42 authored, 21 derived rows (D5-2) - inspect: --json is a groma scanner discover option only (D5-4) - CONTRIBUTING: remove the deleted typescript-scanner verify step (D-sc-07) and the deleted parcel-bytecode test instruction (D-work-06)
TASK-531 tracks the remediation PR required by CONTRIBUTING: actor, entry points, observable result, and per-finding scope.
Groma architecture comparison
|
…comparisons External fork PRs no longer export architecture automatically. Maintainers can review a PR, then choose **Groma architecture → Run workflow** on `main` and enter its number. Same-repository PRs remain automatic; the manual choice applies to one run. Comparison jobs keep the trusted base checked out and fetch the PR head only as Git data, so checkout v7 protection remains enabled. The workflow selects Groma 0.6.6 explicitly through the tested Action version input. README and contributor instructions explain the workflow and reader upgrades. Validation: `bun run check` passes (773 Bun tests, 51 optional skips, plus Node/lint/type checks). The released 0.6.6 CLI exported #113 from a trusted main checkout. The selection command excludes #113 by default and includes its exact head/base when selected manually. Action integration passes with a base checkout and a different exported head. Action change: MrLesk/groma.md-action#5
Record completed validation only. Source commit 6151fce passed CI on Windows, Linux, and macOS in run 37453651554. No code or workflow changes. [skip ci]
|
I checked this PR while looking at Groma for a Gray Cat video. Two security gaps still remain at
The existing web tests passed, including the foreign-host, foreign-origin and direct I ran the 11 changed test files on Bun 1.3.14: 32 tests passed, and two files failed to load because of parser errors in unchanged scanner files. I did not run the declared Bun 1.4.2 version. The two reviewers completed the source review; the live reproductions above were run by Codex. I would fix these gaps before relying on this PR for safe local use. 😸 Generated with The Gray Cat | Orchestrated by OpenAI Codex; reviewed by Codex + Claude in parallel |
Merge origin/main into the audit pull request. plugins/scanners/typescript/src/projects.ts takes main's version: main already reports an absent referenced config (TASK-558) and tests it in test-bun/nested-scanners.test.ts, so the pull request's second implementation and its test file are dropped. src/cli.ts keeps the pull request's catch around a welcome action and main's awaited session close.
Renumber the audit task record from TASK-531 to TASK-581. Main already owns TASK-531 (Publish PR architecture comparisons), so the Backlog CLI could not resolve either record. No other file added by this pull request names the audit task by id.
Restore main's Backlog work source apart from the task id boundary. Field coercion, the SIGKILL escalation and the rewritten stream parser guarded a misbehaving Backlog CLI that nobody reproduced, so main's code returns. The id check and the quoted arguments for the Windows .cmd shim stay: a task id is repository content that returns to a command line. The one remaining test moves to test-bun/backlog-cli.test.ts, beside the other tests of what the adapter sends to the CLI.
Restore the Swift worker's unlimited output buffer. Evidence grows with the source, so main sets no fixed limit, and the shared JVM runner made the same choice. A 256 MB cap would fail a large project instead of protecting it.
Renumber the audit task record from TASK-581 to TASK-591. Ids up to 580 were taken in the shared checkout after the first renumbering, so 581 is no longer safe. The file name, the front matter id and the plan's own mention of the id change; earlier commits keep the subject they were made with.
Restore the repository listing and its pinned symlink expectation. Main lists a symlink that points outside the repository on purpose; its test names Shared.swift for that reason. Dropping such links is undone. A symlink whose target is absent never stopped the listing: the existence filter removes it before its target is resolved. The pull request's try/catch covered only a target vanishing between those two calls, which nobody reproduced, so it goes too. One test beside main's symlink test now states the rule.
Restore docs/relationship-inference.md. The page is a dated record of an investigation. Replacing its counts with today's measurements would make it describe a state it never observed.
Restore relationship row writing and drop the escaping tests. Another open pull request makes one function the owner of table-text escaping, and two schemes must not land. File names that contain a pipe, a closing bracket or a line break stay an open follow-up.
Keep only the export failure test in the CLI action tests. The process-wide module mock replaced the scanner settings reader for every test in the file, so the two welcome status tests built on it shared state; the help test froze the wording of groma draft. All three go. The export failure test stays and now runs in its own temporary repository instead of this checkout. It no longer matches Git's wording.
Drop the two work island tests that ran on a hand-made DOM. The repository's tests have no DOM. The stub recorded what the island wrote into it and the assertions read a field that only the stub has, so they proved the stub's shape rather than what a browser renders. The server-level tests in the file stay.
Listen on the loopback address only. The viewer answered on every network interface, and the Host check could not keep another machine out: a client that is not a browser sends any Host it likes. Bun.serve now binds 127.0.0.1. The printed address stays http://localhost:<port>; a client that resolves localhost to ::1 first falls back to 127.0.0.1. The Host and Origin checks stay for requests a browser sends on behalf of another site. The function is named for what it detects, parses each header once and keeps the loopback names in a set.
Reject a code reference that leaves the repository when the model is built. A stored file such as ../outside.txt was refused by the source route but still read through /world.json?from=<commit>, because the check sat at some readers and not at others. codeReferencesOf already validates every stored code reference, so it now rejects a file that is absolute or has a `..` segment. Every reader loads its architecture through the model, which covers the live map, a commit, a comparison and an export. The per-route checks and src/viewers/web/containment.ts are gone; their removal was committed one step early, in 0587db6. The source-route and export tests give way to one model test of the rule and one route test of the comparison the review named.
Reduce the work island fix to the two places that needed it. A Backlog task id and a status name reached innerHTML. They are now appended as text after the button is built from this module's own markup, which keeps main's button helper, chip and status toggle as they were. The fragment helper, the extracted status button and the exports existed only for the hand-made DOM tests that are gone. Checked in headless Chromium with a task id and a status that carry an img tag with an error handler: main builds two img elements and runs the handler, this version builds none and shows the text.
Pin the four action references the architecture workflow still floated. Main added the comparison job after the pull request pinned its actions, so one file held both styles. The checkout reuses the hash already pinned for v7.0.1 in the same file. upload-artifact v4 and download-artifact v4 were resolved with `gh api repos/actions/<name>/commits/v4`: ea165f8d65b6e75b540449e92b4886f43607fa02 and d3f86a106a0bac45b974a628896c90dbdf5c8093.
Keep the unpaged JSON result in the inspect guide. The correction named the one command that accepts --json but dropped the fact the sentence was there for: that result is printed whole, not paged.
Report a failed welcome action in one place. runInteractiveWelcome caught an action's error itself, and both commands that call it, groma and groma instructions, catch again with the same two lines. The inner catch goes, so the session closes and the caller prints the message and sets exit code 1, as every other command does. Checked in a terminal session: choosing groma scan from the welcome with an unreadable groma/scanners.json prints the parse error alone and exits 1; quitting groma instructions exits 0.
Keep tolerant manifest reading in its one reachable place. The shared entry-point reader parses every manifest and workspace file beside the scanned sources, so one broken package.json, such as a fixture a repository keeps on purpose, failed the JavaScript, TypeScript, React, Vue and Angular scans. It still skips that file with a warning, but only for invalid JSON: any other error is a defect and stays visible. The diagnostics come back with the entries instead of through an argument, and the casts on the parsed manifest are gone. The Angular and React readers return to main. Both read the manifest of a project that typescript-project.ts already selected by parsing that same file, so their catch could not run. The Angular one also described a project named after its directory as unnamed and built the diagnostic path from the working directory. Skipping project references that leave the repository in the Angular reader was not among the kept changes.
Reduce the discovery fix to one check and make its test fail on main. The package search climbs from the declaring file to the filesystem root, so a copy installed above the repository was reported as the project's own. One check on the path that is stored anyway now skips it; the second resolve of an already absolute directory is gone. The test installed the package inside the repository as well, where main finds it first, so it passed without the fix. It now declares the dependency in two packages and installs it for one of them, with the nearest copy for the other above the repository.
Order pins by numeric collation instead of a hand-written comparator. Main read a task id's number as a decimal, so TASK-416.10 came before TASK-416.2. localeCompare with numeric collation compares each run of digits as a number, which is the order Backlog gives subtasks; the route ports already sort ids this way. The fifteen-line comparator and its branch for ids without any number, a case Backlog does not produce, are gone, and the test keeps the dotted ids only.
Name the headers the request gate reads in its comment.
Document that a manifest which is not JSON supplies no entry. The JavaScript scanner page states the entry reader's rules, which the TypeScript, React, Vue and Angular pages refer to.
Describe in the task record what the pull request contains now. The description, acceptance criteria, plan and notes state the result of the clean-up: 19 audit fixes kept, 7 back to main's code, 2 already on main, each with its one-line reason, the tests' rationale and what was verified. The modified-file list names the 28 files that differ from main.
Never open a task's modified file that is named outside the repository. An open task whose modified files hold ../outside.txt made the task diff, served by /task-diff.json and the terminal viewer, return that file. A task record is repository content, like a stored code reference, so the same rule applies and keeps one owner: code-reference.ts exports it as isRepositoryRelativePath and the reader's working-tree read uses it. Such a row reads as absent, the way a missing file does. The rule is about spelling, so the name and the model's message now say "a repository-relative path without a .. segment" instead of promising where the path ends up.
Say exactly what the request gate decides, and test it through fetch. The gate's name and first sentence promised to detect every cross-site request, but a GET or HEAD with a loopback Host passes whatever its Origin. It is now `forbidden`, and its comment lists the three facts: every request needs a loopback Host; only requests other than GET and HEAD are checked for Origin or Referer; a cross-site GET still runs but its page cannot read the answer, so a GET route must never change anything. The web viewer page says the same. `[::1]` leaves the loopback names: the listener is IPv4, so nothing reaches it under that name. The comment on the printed address states what was checked, macOS only. The server tests move to test-bun/web-server.test.ts. Bun's fetch sends a custom Host header, so the raw socket helper and node:net are gone; the shared EMPTY_WORK_SOURCE replaces the local copy; the hook that removed groma/ is gone because the Host check does not need setup mode. The comparison test's title and comment say it is the one deliberate route-level check of the model's rule.
Add the source and document changes that 660db60 describes. That commit carried only the renamed test file: a failed `git add` left src/viewers/web/server.ts and docs/viewers/web/index.md unstaged. They are the `forbidden` gate with its exact comment, the loopback names without `[::1]`, the comment on the printed address, and the matching paragraph on the web viewer page.
Delete the two tests of main's atomic writer. The second passed with a non-atomic writer, because the content it called preserved sat where no writer could touch it; the first repeated test-bun/filesystem-access.test.ts. Neither covered code this pull request changes.
Remove the sentence that says a static export reads tasks. It contradicted the export paragraph this pull request corrected in the same page and the code: an export carries no task data.
Return the Windows shim's argument line to main. With the id check every argument is a fixed word or a checked id, so the added quotes changed nothing; they were never run on Windows and their comment claimed more than quotes do in cmd.exe. The id comment now also says why the first character cannot be a dash. The id test no longer matches the error text: the line that proves the CLI was started once, for the valid id, already shows the refusal.
Report a failed command in one catch around the parser. The root, export and instructions actions each repeated the same two lines. One catch around program.parseAsync() replaces the three and also covers the commands that had none; the actions are main's again. Checked in a terminal session: choosing groma scan from the welcome with an unreadable groma/scanners.json prints the parse error alone and exits 1; quitting groma instructions exits 0.
Label every pinned action with its exact release. Most pins carried the moving major tag, three of which no longer point at the pinned commit, and checkout had two labels for one hash. Each label is now the release tag that upstream puts on that commit, read with git ls-remote --tags. Main's own unlabelled pin of groma.md-action is untouched; no upstream tag names that commit.
Name the manifest diagnostic the way the JavaScript scanner names its own. Its codes are upper snake case, JAVASCRIPT_SOURCE_INVALID and JAVASCRIPT_SOURCE_SCOPE, so the new one is JAVASCRIPT_MANIFEST_INVALID.
Sort task ids with a fixed collation locale. An undefined locale follows the machine, so two machines could order the same pins, and assign their colours, differently.
Record the review round in the task. The description, criteria, plan and notes describe the result after the consolidated review list: the task diff reader's path rule, the exact request gate, the single command catch, main's shim line, labelled pins, the renamed server tests and what was verified.
What this is
An external, adversarial security and correctness audit of groma.md v0.5.0 (HEAD
5882073a), run with a three-pass methodology:Result: 37 triple-validated findings — 28 fixed, 9 reported on this branch. Each finding has its own issue with full evidence, failure scenario, remediation, and validation notes:
Triple-validated: independent discovery (R1), adversarial re-derivation by a separate reviewer given only the claim text (R2), and source reproduction on HEAD 5882073.
Fixed on this branch (28)
Reported for maintainer decision (9)
These are real and confirmed, but the right fix is a design/product decision, so this PR deliberately does not change them:
Fixed by area
maxBuffer: Infinity, out-of-tree dependency version attribution.GromaFileSystem.writechoke point, try/catch on root/export/instructions actions, welcome-loop action errors, draft-relation help text, status-bar/notice contradiction, signal-shutdown close() crash.Amended pinned test (needs review)
test-bun/scanner-source-listing.test.tsincidentally pinned the audited D-sc-03 behavior: its fixture symlinksShared.swiftto a file outside the repository root (a sibling temp dir) and asserted the symlink stays listed. Under the fix, out-of-tree symlinks are excluded (that is the point of D-sc-03), so the expected list was amended to dropShared.swift; the test's actual subject — the in-tree symlink deduped to its listed target — is untouched and still passes.Checks run
bun run checkon this branch: 763 pass / 7 fail (+26 new tests, all passing). The 7 fails are the pre-existing environment-dependent set on this arm64 macOS host with no Java/SwiftSyntax toolchain — 6 Swift (worker needs bundled SwiftParser host modules that ship via CI's setup-swift) and 1 Java (JDK-version-sensitive assertion; verified identical pre-fix at 737/7 with JDK 21 and 763/7 with JDK 27, so none are regressions). CI validates these.Checked and cleared (not filed, for completeness)
Linked task: TASK-531 ("Harden audited surfaces and correct documentation drift") in
backlog/tasks/.